Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cwgrants): Implement x/cwgrants module #527

Merged
merged 31 commits into from
Jan 16, 2024
Merged

feat(cwgrants): Implement x/cwgrants module #527

merged 31 commits into from
Jan 16, 2024

Conversation

fdymylja
Copy link
Contributor

@fdymylja fdymylja commented Jan 2, 2024

No description provided.

Copy link

codecov bot commented Jan 2, 2024

Codecov Report

Attention: 137 lines in your changes are missing coverage. Please review.

Comparison is base (58575f1) 65.61% compared to head (6c3505b) 64.50%.

Files Patch % Lines
x/cwfees/types/cw.go 0.00% 37 Missing ⚠️
x/cwfees/keeper.go 68.05% 15 Missing and 8 partials ⚠️
x/rewards/ante/fee_deduction.go 32.35% 22 Missing and 1 partial ⚠️
x/cwfees/module.go 46.87% 16 Missing and 1 partial ⚠️
x/cwfees/types/msgs.go 0.00% 16 Missing ⚠️
x/cwfees/msg_server.go 7.69% 12 Missing ⚠️
x/cwfees/query_server.go 12.50% 7 Missing ⚠️
x/cwfees/types/codec.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #527      +/-   ##
==========================================
- Coverage   65.61%   64.50%   -1.12%     
==========================================
  Files          80       88       +8     
  Lines        4633     4851     +218     
==========================================
+ Hits         3040     3129      +89     
- Misses       1436     1556     +120     
- Partials      157      166       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fdymylja fdymylja marked this pull request as ready for review January 9, 2024 12:09
@fdymylja fdymylja requested a review from a team as a code owner January 9, 2024 12:09
@fdymylja fdymylja requested a review from spoo-bar January 9, 2024 12:09
# Conflicts:
#	CHANGELOG.md
#	app/app.go
#	app/keepers/keepers.go
#	app/upgrades/latest/upgrades.go
// SudoMsg defines the message sudo enum that is sent to the CosmWasm contract.
type SudoMsg struct {
// CWGrant defines the enum variant of the grant message.
CWGrant *CWGrant `json:"cw_grant"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWGrant to CWFees?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think CWGrant is fine here, whilst the "module name" is CWFees (as it handles fees); what is being sent as request here is a request for a fee grant.

The name is kept as short as possible to avoid extra gas consumption when we traverse the wasm boundary.

Copy link
Contributor

@zanicar zanicar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to change CWGrant to CWFees or are we leaving that for the remediation / refactor cycle? LMKWYT

@fdymylja
Copy link
Contributor Author

@zanicar I would propose we keep as is, and consider renaming this cwfeegrants, to be in line with cosmos sdk's fee grant module, at nomenclature level, I will create an issue.

Copy link
Contributor

@zanicar zanicar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fdymylja fdymylja merged commit 690d44f into main Jan 16, 2024
7 of 9 checks passed
@fdymylja fdymylja deleted the fd/cw_grants branch January 16, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants